Skip to content

feat(valuation): lead-capture + Telegram parity across every valuation caller (chat#1885) - #785

Merged
sweetmantech merged 2 commits into
mainfrom
feat/valuation-lead-capture-parity
Jul 23, 2026
Merged

feat(valuation): lead-capture + Telegram parity across every valuation caller (chat#1885)#785
sweetmantech merged 2 commits into
mainfrom
feat/valuation-lead-capture-parity

Conversation

@sweetmantech

@sweetmantech sweetmantech commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What

The 💰 Valuation lead Attio upsert + team Telegram ping was marketing-frontend-only: only the funnel's client-side captureRunLeadPOST /api/valuation/lead (a marketing route) fired it. Chat-triggered valuations (chat/lib/valuation/runValuationPOST /api/valuation) and direct-api calls create no Attio lead and no alert.

This lifts lead-capture into runValuationHandler — the handler that owns the POST /api/valuation milestone — so every caller (chat, direct api, marketing funnel) captures a lead + fires one team alert. It mirrors the merged valuation-email pattern (#773): a deferred after() once the catalog is materialized and the band computed, best-effort, never blocking the response.

Closes the "Valuation lead-capture + Telegram parity across entry points" item in recoupable/chat#1885.

How

  • Port the Attio primitives (lib/attio/{request,assertPersonByEmail,createNote,isRecordInList,addRecordToList}) and the lead orchestration (upsertValuationLead, leadAttributes, leadNoteContent) verbatim from marketing (api had Telegram but no Attio). Added a whole-dollar lib/format/usd.
  • New captureValuationLead orchestrator — the server-side equivalent of the marketing /api/valuation/lead route body, with two deliberate differences:
    • resolves the owner's email from the account (selectAccountEmails({ accountIds })), never the request body;
    • the band arrives in api shape ({ low, mid, high }) → maps mid to the lead's central;
    • includes the measured lifetimeStreams (aggregate.totalStreams) that the funnel's client-side path never had — a richer lead.
  • Wire it into runValuationHandler as a second after(() => …) beside the valuation-email one.

Attio behavior is unchanged from the funnel: assert Person by email → append a chronology Note every run → add to the Valuation Leads pipeline at New only if absent (re-runs never duplicate the card).

No contract change

POST /api/valuation still returns { catalog, band, songs_measured }. Lead-capture + Telegram are side-effects (exactly like the valuation email in #773), so there is no OpenAPI/docs change — accuracy over symmetry, we don't document a response the API doesn't emit.

De-dupe

The plan is: api is the single source, and marketing drops its now-duplicate client-side ping in a follow-up marketing PR so a funnel valuation fires exactly one lead. Recommended merge order: this PR, then the marketing removal back-to-back (a brief double-fire window for funnel valuations during rollout is cosmetic — one extra Attio note + one extra ping).

⚠️ Deploy prerequisite

The api Vercel project needs ATTIO_API_KEY set (Preview + Production) for the lead to actually write to Attio — upsertValuationLead no-ops with a logged ATTIO_API_KEY not configured otherwise. Telegram creds (TELEGRAM_CHAT_ID + bot token) already exist in api. Verify/set ATTIO_API_KEY on the api project before merging.

Tests

TDD, RED→GREEN per unit. 45 tests across lib/attio, lib/format, lib/valuation green; full suite + tsc --noEmit (my files clean; the 3 lib/trigger errors pre-exist on main) + eslint all clean.

  • Attio primitives: URL/method/auth/body + best-effort no-throw on failure.
  • upsertValuationLead: attributes, chronology note, pipeline dedup, missing-key + assert-error paths.
  • captureValuationLead: email-from-account, mid → central mapping, Telegram format + Attio deep link, no-email skip, Attio-fail-still-pings, Telegram-fail-never-throws, optional-signal omission.

Live real-send verification against the preview (chat + direct-api valuation each → one Attio lead + one Telegram ping; funnel still one after the marketing PR) to follow in a PR comment once ATTIO_API_KEY is on the preview.

🤖 Generated with Claude Code


Summary by cubic

Moves valuation lead capture into the API runValuationHandler so chat, direct API, and marketing callers all create one Attio lead and one Telegram alert. Runs after valuation completes and never blocks the response, addressing chat#1885.

  • New Features

    • Added captureValuationLead: resolves owner email from the account, maps valueBand.midcentral, adds lifetimeStreams and optional followerCount.
    • Attio: upserts Person, adds a markdown note, and dedupes a “Valuation Leads” pipeline card; Telegram ping includes an Attio deep link.
    • Ported Attio helpers (lib/attio/*) and lib/format/usd; wired via after() in runValuationHandler. No POST /api/valuation response change.
  • Migration

    • Set ATTIO_API_KEY for the API (Preview + Production) before deploy.
    • Remove the marketing client /api/valuation/lead call after rollout to avoid double alerts.

Written for commit 066de3e. Summary will update on new commits.

Review in cubic

…ller (chat#1885)

The "💰 Valuation lead" Attio upsert + team Telegram ping was
marketing-frontend-only: only the funnel's client-side `captureRunLead`
→ `/api/valuation/lead` path fired it. Chat-triggered valuations and
direct-api calls (both hit the shared `POST /api/valuation`) created no
Attio lead and no alert.

Lift lead-capture into `runValuationHandler`, the milestone owner, so
every caller captures a lead. Mirrors the valuation-email pattern:
`after()` after the catalog is materialized + band computed, best-effort,
never blocks the response.

- Port the Attio primitives (`lib/attio/*`) + the lead orchestration
  (`upsertValuationLead`, `leadAttributes`, `leadNoteContent`) verbatim
  from marketing, plus a whole-dollar `lib/format/usd`.
- New `captureValuationLead`: resolves the owner's email from the account
  (`selectAccountEmails`), never the body; maps the api band `mid` →
  lead `central`; includes the measured `lifetimeStreams` the funnel's
  client path never had.
- No `POST /api/valuation` response-contract change — this is a
  side-effect like the valuation email (api#773), so no docs change.

Marketing removes its now-duplicate client-side ping in a follow-up PR
so a funnel valuation fires exactly one lead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 23, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api Ready Ready Preview Jul 23, 2026 9:21pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@sweetmantech, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 28955e77-8e12-445b-8ba2-bfbf01508066

📥 Commits

Reviewing files that changed from the base of the PR and between ce2357e and 066de3e.

⛔ Files ignored due to path filters (9)
  • lib/attio/__tests__/addRecordToList.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/attio/__tests__/assertPersonByEmail.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/attio/__tests__/createNote.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/attio/__tests__/isRecordInList.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/format/__tests__/usd.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/valuation/__tests__/captureValuationLead.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/valuation/__tests__/leadAttributes.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/valuation/__tests__/leadNoteContent.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
  • lib/valuation/__tests__/upsertValuationLead.test.ts is excluded by !**/*.test.*, !**/__tests__/** and included by lib/**
📒 Files selected for processing (12)
  • lib/attio/addRecordToList.ts
  • lib/attio/assertPersonByEmail.ts
  • lib/attio/createNote.ts
  • lib/attio/isRecordInList.ts
  • lib/attio/request.ts
  • lib/format/usd.ts
  • lib/valuation/captureValuationLead.ts
  • lib/valuation/leadAttributes.ts
  • lib/valuation/leadNoteContent.ts
  • lib/valuation/runValuationHandler.ts
  • lib/valuation/upsertValuationLead.ts
  • lib/valuation/valuationLeadInput.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/valuation-lead-capture-parity

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9 issues found across 21 files

Confidence score: 2/5

  • In lib/attio/isRecordInList.ts, membership-read errors are currently interpreted as “not in list,” so upsertValuationLead can call addRecordToList and create duplicate Attio pipeline cards during outages or malformed responses — treat lookup/request failures as explicit failures (or a distinct unknown state) instead of absence.
  • In lib/attio/request.ts, rejected network fetches can escape the helper, violating the best-effort contract and aborting the lead flow before downstream handling — catch transport-level rejections in the helper/wrappers and return the existing fail-safe shape.
  • In lib/valuation/captureValuationLead.ts (with lib/valuation/upsertValuationLead.ts), transient Attio failures can suppress the required Telegram alert, and missing record_id can still return { success: true }, hiding a failed upsert — catch rejected upserts locally and treat missing IDs as a failure path so alerting and reporting stay accurate.
  • In lib/attio/__tests__/addRecordToList.test.ts and lib/attio/__tests__/createNote.test.ts, console.error spies may leak when assertions fail, causing cross-test contamination and flaky diagnostics — move spy restoration to afterEach/finally for guaranteed cleanup.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="lib/attio/request.ts">

<violation number="1" location="lib/attio/request.ts:8">
P3: This Attio request helper is exported as `attioFetch` but lives in `request.ts`, which breaks the repository's required function-to-file naming convention. Moving it to `lib/attio/attioFetch.ts` keeps imports and module discovery consistent.</violation>

<violation number="2" location="lib/attio/request.ts:9">
P1: A network rejection from Attio escapes the low-level helper and aborts the lead flow before its Telegram alert, despite the wrappers and `upsertValuationLead` promising best-effort behavior. Handling fetch rejections in the helper or each operation wrapper would let the flow log the Attio failure and continue.</violation>
</file>

<file name="lib/valuation/captureValuationLead.ts">

<violation number="1" location="lib/valuation/captureValuationLead.ts:55">
P2: A transient Attio network failure suppresses the Telegram alert, despite this flow requiring the alert to continue after Attio errors. Catch a rejected `upsertValuationLead` locally and convert it to the existing failed outcome before sending.</violation>
</file>

<file name="lib/attio/isRecordInList.ts">

<violation number="1" location="lib/attio/isRecordInList.ts:13">
P2: A transient fetch rejection currently escapes despite this helper's best-effort contract, so lead capture can reject instead of treating the membership lookup as absent. Catch request/response parsing failures and validate `data.data` before calling `.some()`.</violation>

<violation number="2" location="lib/attio/isRecordInList.ts:16">
P1: An Attio membership-read failure is treated as “not in the list,” so `upsertValuationLead` proceeds to `addRecordToList` and can create duplicate pipeline cards during an Attio outage or malformed response. Returning a separate verification error (or otherwise making the caller skip the add when the read cannot be trusted) would preserve deduplication.</violation>
</file>

<file name="lib/attio/__tests__/addRecordToList.test.ts">

<violation number="1" location="lib/attio/__tests__/addRecordToList.test.ts:33">
P3: A failed assertion in this test can leak the `console.error` spy into later tests because `mockRestore()` is only reached on the success path. Moving spy cleanup into an `afterEach`/`finally` path would preserve test isolation.</violation>
</file>

<file name="lib/attio/__tests__/createNote.test.ts">

<violation number="1" location="lib/attio/__tests__/createNote.test.ts:37">
P3: Spy on `console.error` is restored inline in the test body but not in `afterEach`, so a test failure before the restore line leaves the spy active across tests. Move restoration into `afterEach` so it runs regardless of pass/fail.</violation>
</file>

<file name="lib/format/usd.ts">

<violation number="1" location="lib/format/usd.ts:7">
P3: `NaN` and `±Infinity` pass through to produce `"$NaN"`, `"$∞"`, and `"$-∞"` — none of which are valid USD strings. A guard at the top of the function would make the contract defensive against unexpected inputs.</violation>
</file>

<file name="lib/valuation/upsertValuationLead.ts">

<violation number="1" location="lib/valuation/upsertValuationLead.ts:40">
P2: When `assertPersonByEmail` returns a 200 OK response but the JSON body is missing `data.data.id.record_id`, `recordId` is `undefined`. The `return { success: true }` on the `!recordId` path means the code reports success without creating a note, adding a pipeline card, or logging anything — effectively dropping the lead silently.

The JSON-parsing catch in `assertPersonByEmail` (`res.json().catch(() => null)`) means this path is reachable whenever the Attio API returns a 200 with an unexpected or malformed body. While unlikely, this is the one path where a lead can be lost without observability.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant Client as Any Caller (Chat, API, Marketing)
    participant Route as POST /api/valuation
    participant Handler as runValuationHandler
    participant Catalog as Catalog Service
    participant DB as Supabase (Account/Artist)
    participant Email as sendValuationReportEmail
    participant Capture as captureValuationLead (NEW)
    participant Upsert as upsertValuationLead (NEW)
    participant Attio as Attio REST API
    participant Telegram as Telegram Bot

    Note over Client,Telegram: Valuation Request – Happy Path (with lead capture)

    Client->>Route: POST /api/valuation
    Route->>Handler: delegate
    Handler->>Catalog: compute band, aggregate streams
    Catalog-->>Handler: valueBand {low,mid,high}, totalStreams, etc.
    Handler->>DB: fetch account/artist info
    DB-->>Handler: accountId, artist details

    Note over Handler: Response sent immediately, side effects via after()
    Handler-->>Route: 200 { catalog, band, songs_measured }

    alt after() – Valuation Email (existing)
        Handler->>Email: sendValuationReportEmail()
    end

    alt after() – Lead Capture (NEW)
        Handler->>Capture: captureValuationLead({accountId, artistName, artistId, valueBand, lifetimeStreams, followerCount})
        Capture->>DB: selectAccountEmails({accountIds})
        DB-->>Capture: emailRow[]

        alt No email found
            Note over Capture: Skip entire lead capture
            Capture-->>Handler: return
        else Email found
            Capture->>Capture: map valueBand.mid → central, build lead object
            Capture->>Upsert: upsertValuationLead(lead)

            alt ATTIO_API_KEY missing
                Upsert-->>Capture: { success:false, error:"ATTIO_API_KEY not configured" }
                Note over Capture: Log error, proceed to Telegram
            else Key present
                Note over Upsert: Attio Person upsert by email
                Upsert->>Attio: PUT /objects/people/records?matching_attribute=email_addresses
                alt Assert succeeded
                    Attio-->>Upsert: { record_id: "rec_1" }
                    Upsert->>Attio: POST /notes (chronology note)
                    Attio-->>Upsert: ok
                    Upsert->>Attio: GET /objects/people/{recordId}/entries
                    Attio-->>Upsert: { data: entries[] }
                    alt Record not in list
                        Upsert->>Attio: POST /lists/valuation_leads/entries (stage: New)
                        Attio-->>Upsert: ok
                    else Already in pipeline
                        Note over Upsert: Skip – no duplicate card
                    end
                    Upsert-->>Capture: { success:true, recordUrl:"https://app.attio.com/…/rec_1/overview" }
                else Assert failed
                    Attio-->>Upsert: 400/500
                    Upsert-->>Capture: { success:false, error:"…" }
                    Note over Capture: Log Attio error, still send Telegram (without link)
                end
            end

            Capture->>Telegram: sendMessage("💰 Valuation lead\nEmail: {email}\nArtist: {...}\nEstimated: $54,600,000 (range $37.5M–$76.8M)\nAttio: {link}")
            alt Telegram fails
                Note over Capture: Catch error, log, never throw
            end
            Capture-->>Handler: (void)
        end
    end

    Note over Client,Telegram: Result: lead captured + alert fired for every caller
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

const res = await attioFetch(`/objects/${object}/records/${recordId}/entries`, {
method: "GET",
});
if (!res.ok) return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: An Attio membership-read failure is treated as “not in the list,” so upsertValuationLead proceeds to addRecordToList and can create duplicate pipeline cards during an Attio outage or malformed response. Returning a separate verification error (or otherwise making the caller skip the add when the read cannot be trusted) would preserve deduplication.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/attio/isRecordInList.ts, line 16:

<comment>An Attio membership-read failure is treated as “not in the list,” so `upsertValuationLead` proceeds to `addRecordToList` and can create duplicate pipeline cards during an Attio outage or malformed response. Returning a separate verification error (or otherwise making the caller skip the add when the read cannot be trusted) would preserve deduplication.</comment>

<file context>
@@ -0,0 +1,20 @@
+  const res = await attioFetch(`/objects/${object}/records/${recordId}/entries`, {
+    method: "GET",
+  });
+  if (!res.ok) return false;
+  const data = await res.json().catch(() => null);
+  const entries: Array<{ list_id?: string; id?: { list_id?: string } }> = data?.data ?? [];
</file context>

Comment thread lib/attio/request.ts
* Reads ATTIO_API_KEY at call time (server-only); returns the raw Response.
*/
export function attioFetch(path: string, init?: RequestInit): Promise<Response> {
return fetch(`${ATTIO_BASE_URL}${path}`, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: A network rejection from Attio escapes the low-level helper and aborts the lead flow before its Telegram alert, despite the wrappers and upsertValuationLead promising best-effort behavior. Handling fetch rejections in the helper or each operation wrapper would let the flow log the Attio failure and continue.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/attio/request.ts, line 9:

<comment>A network rejection from Attio escapes the low-level helper and aborts the lead flow before its Telegram alert, despite the wrappers and `upsertValuationLead` promising best-effort behavior. Handling fetch rejections in the helper or each operation wrapper would let the flow log the Attio failure and continue.</comment>

<file context>
@@ -0,0 +1,17 @@
+ * Reads ATTIO_API_KEY at call time (server-only); returns the raw Response.
+ */
+export function attioFetch(path: string, init?: RequestInit): Promise<Response> {
+  return fetch(`${ATTIO_BASE_URL}${path}`, {
+    ...init,
+    headers: {
</file context>


// Attio is the system of record. Don't block the ping on an Attio error —
// the valuation already succeeded — but log a dropped lead so it's observable.
const attio = await upsertValuationLead(lead);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A transient Attio network failure suppresses the Telegram alert, despite this flow requiring the alert to continue after Attio errors. Catch a rejected upsertValuationLead locally and convert it to the existing failed outcome before sending.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/valuation/captureValuationLead.ts, line 55:

<comment>A transient Attio network failure suppresses the Telegram alert, despite this flow requiring the alert to continue after Attio errors. Catch a rejected `upsertValuationLead` locally and convert it to the existing failed outcome before sending.</comment>

<file context>
@@ -0,0 +1,73 @@
+
+    // Attio is the system of record. Don't block the ping on an Attio error —
+    // the valuation already succeeded — but log a dropped lead so it's observable.
+    const attio = await upsertValuationLead(lead);
+    if (!attio.success) {
+      console.error("[valuation/lead] Attio enrichment failed:", attio.error);
</file context>

recordId: string,
listId: string,
): Promise<boolean> {
const res = await attioFetch(`/objects/${object}/records/${recordId}/entries`, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A transient fetch rejection currently escapes despite this helper's best-effort contract, so lead capture can reject instead of treating the membership lookup as absent. Catch request/response parsing failures and validate data.data before calling .some().

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/attio/isRecordInList.ts, line 13:

<comment>A transient fetch rejection currently escapes despite this helper's best-effort contract, so lead capture can reject instead of treating the membership lookup as absent. Catch request/response parsing failures and validate `data.data` before calling `.some()`.</comment>

<file context>
@@ -0,0 +1,20 @@
+  recordId: string,
+  listId: string,
+): Promise<boolean> {
+  const res = await attioFetch(`/objects/${object}/records/${recordId}/entries`, {
+    method: "GET",
+  });
</file context>

const today = new Date().toISOString().slice(0, 10);
const { recordId, error } = await assertPersonByEmail(leadAttributes(lead, today));
if (error) return { success: false, error };
if (!recordId) return { success: true };

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When assertPersonByEmail returns a 200 OK response but the JSON body is missing data.data.id.record_id, recordId is undefined. The return { success: true } on the !recordId path means the code reports success without creating a note, adding a pipeline card, or logging anything — effectively dropping the lead silently.

The JSON-parsing catch in assertPersonByEmail (res.json().catch(() => null)) means this path is reachable whenever the Attio API returns a 200 with an unexpected or malformed body. While unlikely, this is the one path where a lead can be lost without observability.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/valuation/upsertValuationLead.ts, line 40:

<comment>When `assertPersonByEmail` returns a 200 OK response but the JSON body is missing `data.data.id.record_id`, `recordId` is `undefined`. The `return { success: true }` on the `!recordId` path means the code reports success without creating a note, adding a pipeline card, or logging anything — effectively dropping the lead silently.

The JSON-parsing catch in `assertPersonByEmail` (`res.json().catch(() => null)`) means this path is reachable whenever the Attio API returns a 200 with an unexpected or malformed body. While unlikely, this is the one path where a lead can be lost without observability.</comment>

<file context>
@@ -0,0 +1,59 @@
+  const today = new Date().toISOString().slice(0, 10);
+  const { recordId, error } = await assertPersonByEmail(leadAttributes(lead, today));
+  if (error) return { success: false, error };
+  if (!recordId) return { success: true };
+
+  const recordUrl = `https://app.attio.com/${ATTIO_WORKSPACE}/person/${recordId}/overview`;
</file context>

Comment thread lib/attio/request.ts
* operation module (assertPersonByEmail, createNote, …) stays a thin wrapper.
* Reads ATTIO_API_KEY at call time (server-only); returns the raw Response.
*/
export function attioFetch(path: string, init?: RequestInit): Promise<Response> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This Attio request helper is exported as attioFetch but lives in request.ts, which breaks the repository's required function-to-file naming convention. Moving it to lib/attio/attioFetch.ts keeps imports and module discovery consistent.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/attio/request.ts, line 8:

<comment>This Attio request helper is exported as `attioFetch` but lives in `request.ts`, which breaks the repository's required function-to-file naming convention. Moving it to `lib/attio/attioFetch.ts` keeps imports and module discovery consistent.</comment>

<file context>
@@ -0,0 +1,17 @@
+ * operation module (assertPersonByEmail, createNote, …) stays a thin wrapper.
+ * Reads ATTIO_API_KEY at call time (server-only); returns the raw Response.
+ */
+export function attioFetch(path: string, init?: RequestInit): Promise<Response> {
+  return fetch(`${ATTIO_BASE_URL}${path}`, {
+    ...init,
</file context>

addRecordToList("valuation_leads", "people", "rec_1", { stage: ["New"] }),
).resolves.toBeUndefined();
expect(errSpy).toHaveBeenCalled();
errSpy.mockRestore();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: A failed assertion in this test can leak the console.error spy into later tests because mockRestore() is only reached on the success path. Moving spy cleanup into an afterEach/finally path would preserve test isolation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/attio/__tests__/addRecordToList.test.ts, line 33:

<comment>A failed assertion in this test can leak the `console.error` spy into later tests because `mockRestore()` is only reached on the success path. Moving spy cleanup into an `afterEach`/`finally` path would preserve test isolation.</comment>

<file context>
@@ -0,0 +1,35 @@
+      addRecordToList("valuation_leads", "people", "rec_1", { stage: ["New"] }),
+    ).resolves.toBeUndefined();
+    expect(errSpy).toHaveBeenCalled();
+    errSpy.mockRestore();
+  });
+});
</file context>


it("logs but does not throw on failure", async () => {
vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response("bad", { status: 500 })));
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Spy on console.error is restored inline in the test body but not in afterEach, so a test failure before the restore line leaves the spy active across tests. Move restoration into afterEach so it runs regardless of pass/fail.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/attio/__tests__/createNote.test.ts, line 37:

<comment>Spy on `console.error` is restored inline in the test body but not in `afterEach`, so a test failure before the restore line leaves the spy active across tests. Move restoration into `afterEach` so it runs regardless of pass/fail.</comment>

<file context>
@@ -0,0 +1,42 @@
+
+  it("logs but does not throw on failure", async () => {
+    vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response("bad", { status: 500 })));
+    const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+    await expect(createNote(note)).resolves.toBeUndefined();
+    expect(errSpy).toHaveBeenCalled();
</file context>

Comment thread lib/format/usd.ts
* from `lib/emails/valuationReport/formatCompactUsd` (which compacts to "$54.6M"
* for the email) and `lib/stripe/formatUsd` (which takes cents, for billing).
*/
export function usd(n: number): string {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: NaN and ±Infinity pass through to produce "$NaN", "$∞", and "$-∞" — none of which are valid USD strings. A guard at the top of the function would make the contract defensive against unexpected inputs.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/format/usd.ts, line 7:

<comment>`NaN` and `±Infinity` pass through to produce `"$NaN"`, `"$∞"`, and `"$-∞"` — none of which are valid USD strings. A guard at the top of the function would make the contract defensive against unexpected inputs.</comment>

<file context>
@@ -0,0 +1,9 @@
+ * from `lib/emails/valuationReport/formatCompactUsd` (which compacts to "$54.6M"
+ * for the email) and `lib/stripe/formatUsd` (which takes cents, for billing).
+ */
+export function usd(n: number): string {
+  return `$${Math.round(n).toLocaleString("en-US")}`;
+}
</file context>

@sweetmantech

Copy link
Copy Markdown
Contributor Author

Live-verified end-to-end (preview + ATTIO_API_KEY set)

Ran a real POST /api/valuation (direct-api path — same runValuationHandler the chat path uses) against the preview after ATTIO_API_KEY was added to the api project.

Result → all three side effects fire, non-blocking:

  • 200 in ~33s (band mid $3,570,885); the after() hooks run post-response, so nothing blocks the valuation.

  • Valuation email still sends (no regression): email_send_log row status=sent, resend_id cdf39e9c, to sweetmantech@gmail.com.

  • Attio lead created: upsertValuationLead created/linked a real Attio Person (deep-linked in the alert below).

  • Team Telegram ping fired (from the internal channel):

    💰 Valuation lead
    Email: sweetmantech@gmail.com
    Artist: Rick Astley
    Estimated catalog value: $3,570,885 (range $2,452,531–$5,022,784)
    Attio: app.attio.com/recoup/person/0abd9eca…/overview

    The band $3,570,885 matches the API's band.mid exactly, and the presence of a real Attio Person URL confirms the upsert landed before the ping was built.

Owner email resolved from the account (selectAccountEmails), not the request body — as designed. This closes the parity gap: chat + direct-api valuations now capture an Attio lead + alert the team, which previously only happened for marketing-funnel valuations.

Merge order reminder: api#785 → marketing#53. #53 removes the funnel's client-side ping so a marketing valuation fires exactly once (with #785 merged first, a marketing valuation briefly double-fires until #53 lands). CI green here.

@sweetmantech

Copy link
Copy Markdown
Contributor Author

Attio state verified directly (via Attio REST API)

Followed up the Telegram evidence by querying Attio directly for the lead the api wrote:

  • Person 0abd9eca-…email_addresses: [sweetmantech@gmail.com]. Created 2026-06-17, i.e. it pre-existed, so upsertValuationLead correctly asserted (upserted) the existing Person by email rather than duplicating.
  • On the Valuation Leads list (entry 9339871a) — and the two runs today did not create a duplicate entry, confirming the isRecordInList dedup works.
  • Two fresh notes written today (one per valuation run), matching my two POST /api/valuation calls:
    • Catalog valuation — Rick Astley (2026-07-23) @ 21:24:31Z and @ 21:31:23Z
    • each: "Valued Rick Astley at $3,570,885 (range $2,452,531–$5,022,784). Lifetime streams: 3,429,421,683. Followers: 1,585,108."

The $3,570,885 in the note matches the API band.mid and the Telegram ping exactly.

Net: with ATTIO_API_KEY set, a direct-api valuation (same handler as chat) now upserts the Attio Person, keeps it on the Valuation Leads pipeline, writes a run note, and pings Telegram — all resolved from the account's email, all non-blocking, no regression to the valuation email. Parity gap closed. Ready to merge (order: api#785 → marketing#53).

@sweetmantech
sweetmantech merged commit 96959eb into main Jul 23, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant